Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[#9] 공통 dialog 컴포넌트 및 아이디어 개요 dialog #12

Merged
merged 14 commits into from
May 12, 2024

Conversation

03hoho03
Copy link
Contributor

@03hoho03 03hoho03 commented May 10, 2024

🧑🏻‍💻 작업 내용

dialog 공통 컴포넌트를 개발하였습니다.
사용 예시로 아이디어 개요 dialog 를 개발하였습니다.

+추가

  • props 에 css props 을 사용해 css 를 적용할 수 있습니다.
    예시
    image

🚀 중요한 포인트

아이디어 개요 dialog
변경전:
image
변경후:
image
최대한 figma 디자인과 가깝게 수정했습니다. (줄간격, 문자간격, 문단간격 등)

styled 구성요소과 styled component 사용 방식에 대해 이상한점이 있으면 피드백 해주세요.

  • 추가
    주의: theme 의 grey 값의 범위가 변하였습니다. 값의 추가로 인해 grey500이 grey400 으로 변하였습니다.
    공통 컴포넌트 제작방식을 변경하였습니다. 참고블로그

Copy link

@chaduhwan chaduhwan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

고생하셨습니다 !

Copy link
Member

@hanseulhee hanseulhee left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

고생하셨습니다! 🔥🔥🔥

style 요소들 가능한 theme에 추가하면서 이를 활용하는 게 좋을 것 같습니다.
그리고 Dialog에 들어가는 글 내용들이 많은데 한 주제의 Dialog마다 줄마다 컴포넌트를 불러와서 작성하는 게 좋은 방법일까요 ?? markdown 형식으로 이를 관리한다면 더 쉽게 글을 쓸 수도 있을 것 같다는 생각이 드는데 의견 궁금합니다 👍🏻👍🏻

src/components/IbulDialog/IdeaOutline.tsx Outdated Show resolved Hide resolved
src/components/common/Dialog/index.ts Outdated Show resolved Hide resolved
src/components/common/Dialog/Title.tsx Outdated Show resolved Hide resolved
src/components/common/Dialog/Description.tsx Outdated Show resolved Hide resolved
src/components/common/Dialog/Overlay.tsx Show resolved Hide resolved
src/components/common/Dialog/Title.tsx Outdated Show resolved Hide resolved
src/components/IbulDialog/IdeaOutline.tsx Show resolved Hide resolved
src/components/IbulDialog/IdeaOutline.tsx Outdated Show resolved Hide resolved
@hanseulhee hanseulhee linked an issue May 10, 2024 that may be closed by this pull request
2 tasks
)
}

export default IdeaOutline

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

export default 를 쓰면 자동완성이나 ctrl + click할때 잘 동작하지 않아서 요즘은 export const로 쓰는 편입니다~

https://dev.to/phuocng/avoid-using-default-exports-a1c

Comment on lines 7 to 9
fontSize?: number
fontWeight?: 300 | 400 | 700 | 800
textAlign?: 'left' | 'center' | 'right'

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

import type { CSSProp } from 'styled-components';

declare module 'react' {
  interface Attributes {
    css?: CSSProp;
  }
}

다음 문법을 styled.d.ts에 이용하면 styled components의 css props를 사용할 수 있는데 보통 공통 컴포넌트를 구현할때는 font-size, font-weight, text-align등 처럼 이미 있는 css 속성은 css props를 통해 열어두고 커스텀 한 속성만을 component의 props로 주어서 처리합니다.

interface ButtonProps {
  fullWidth: boolean; // 사용시 width: 100%로 잡아줌
}

<Button css={{fontSize: '18px', fontWeight: 700 }} fullWidth />

예를 들어 이렇게 fullWidth같이 기존에 없는 속성은 props로 열어두고 이미 존재하는 속성은 css props으로 처리합니다.

Comment on lines +11 to +13
<DialogPrimitive.Trigger className={className} {...props}>
{children}
</DialogPrimitive.Trigger>

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이 dot notation 방식이 잘되나 의문.. 이군용~ nextjs app router에서 dot notation을 쓰면 잘 안되는 이슈가 있어서요

vercel/next.js#51593

@03hoho03
Copy link
Contributor Author

03hoho03 commented May 11, 2024

수정하거나 추가 push 가 일어날때는 알람이 안가서 직접 코멘트를 남깁니다.

  • 절대경로 변경
  • color 및 fontSize theme 의 속성 값으로 변경
  • mdx 적용
  • description default 12px 적용
  • closeBtn 공통 button 적용
    mdx 는 적용하지 않았습니다. (추후 검토)
    button에 대해 수정사항이 예상되어 아직 적용하지 않았습니다. 추후 변경 예정입니다.
    변경사항 확인해보시구 코멘트 남겨주세요!!!
    공통 컴포넌트 작성 방식 #16 discussion 도 한번씩들 확인해주세요

const DialogTitleSizeStyles = css<StyledTitleProps>`
${({ size }) => css`
font-size: ${DialogTitleSizes[size].fontSize};
font-family: ${DialogTitleSizes[size].fontFamily};
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

나눔글꼴 이렇게 해야 하다니 ... 왜지 ..?!

src/components/IbulDialog/IdeaOutline.tsx Outdated Show resolved Hide resolved
src/components/common/Dialog/Title.tsx Outdated Show resolved Hide resolved
Comment on lines 17 to +19
fontWeight: {
light: 300,
normal: 500,
normal: 400, // regular
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이쪽은 그럼 없어도 되는 거 아닌가용 ??

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

당장은 없어두 될꺼같은데 그냥 냅뒀습니다... 삭제할까요??

@03hoho03
Copy link
Contributor Author

  • theme 의 기존 fontWeight 는 일단은 남겨뒀습니다
  • fontWeight를 적용시킬 다른 방법이 생각나지 않아 일단 font family로 바뀌게 하였습니다.
  • figma 디자인에 최대한 맞춰 css 를 변경하였습니다.
  • 결정된 컴포넌트 작성 순서 컨벤션에 맞춰 코드를 수정하였습니다.
    확인 해보시구 이상한거 있으면 말씀해주시구 없으시면 assign 부탁드리겠습니다 !!

Copy link
Member

@hanseulhee hanseulhee left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

고생하셨슴니다 👍🏻👍🏻👍🏻 버튼은 수정해 제가 옮겨두겠습니당

@03hoho03 03hoho03 merged commit 7703c0e into main May 12, 2024
2 checks passed
@03hoho03 03hoho03 deleted the feat/#9 branch May 12, 2024 09:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

[feat] 공통 dialog 컴포넌트
4 participants